-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add static return type #5062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add static return type #5062
Conversation
cc @muglug @nicolas-grekas I believe you were interested in having this. |
returning
if we want to support it, it could be allowed as part of union types: Thank you for opening this! \o/ |
interface DateTimeInterface {
public function add(DateTimeInterval $interval) : static;
public static function createFromFormat(string $format , string $time [, DateTimeZone $timezone ]) : static;
// etc.
} i.e. altering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potentially to be added as invalid scenario: static
in combination of a final
class:
final class A
{
function foo(): static { /* ... */ } // is this valid, or an error?
}
Also, what happens when self
is used in a child class that overrides a method that returns static
?
class A {
function foo(): static { /* ... */ }
}
class B extends A {
function foo(): self { /* ... */ }
}
Similar with interfaces:
interface A {
function foo(): static;
}
class B implements A {
function foo(): self { /* ... */ } // valid or error?
}
I wonder if Hack uses |
this looks valid to me, especially considering your next case:
looks invalid to me, as it breaks the promise of the base class, stating that the method returns the same type as the object's class makes sense? |
I agree with Nicolas, it seems ok for me.
Maybe I misinterpret LSP, but this example also seems ok for me: returning a child class ( |
Replacing A replacement from |
the danger comes here: class A {
public static function getInstance() : static {
return new static();
}
public function foo() : void {}
public static function getInstanceAndCall() : static {
$i = static::getInstance();
$i->foo();
return $i;
}
}
class B extends A {
public static function getInstance() : self {
return new self();
}
}
class C extends B {} calling |
@muglug Ok, now I see. Thanks for the explanation. I was only considering the most basic scenario (when there is only one child class) |
I'm all for this. I was working with @sgolemon on this as an idea at the end of last year. https://phpc.social/@pollita/103256122403635150 Basic description: https://gist.github.com/simensen/729c4ac9056b06d60038377f59a92cfe Sara's implementation: master...sgolemon:static-return I haven't had time to circle back and try to get an RFC up. Now I guess I don't have to? :) Thanks for the work on this!!! Let me know how I can help champion this. |
LGTM, I've wanted this for return types for a while. Some phpt tests of traits would be useful in validating the implementation, e.g.
(I've had a lot of issues with writing code analyzing traits in the past) |
|
||
object(A)#3 (0) { | ||
} | ||
Return value of A::test2() must be an instance of static, instance of A returned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor comment that can be fixed in subsequent PRs for 8.0-dev if needed:
This might be confusing at runtime in more complicated code - the error message gives no indication of what static
is. (e.g. if a method returning static has incorrect caching based on $obj->getKey() which can be reused)
An error message mentioning what the current class scope is may save debugging time, e.g. Return value of A::test2() must be an instance of static (currently static is %s(class/trait/interface) B), instance of A returned
Also, what should happen for code such as this? I assume it should be the same TypeError message, because instanceof
only applies to classes and interfaces. (this is also a potential test case)
trait T {
public static function f() : static {
return new A();
}
}
class A {
use T;
}
T::f();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the implementation to display the type static
resolves to instead. We already do this for self
and parent
, so it makes sense to me to treat static
the same way.
Future scope - method cascading:
What do you think about to implement it like the void, but instead of returning nothing return Syntax sugar in examples:
Reflection should behave exactly the same as if the return type is defined as |
9dbb1e3
to
85f0328
Compare
As of PHP 8.0, `static` can be used as a return type for function declarations. Refs: * https://wiki.php.net/rfc/static_return_type * php/php-src#5062 * php/php-src@4344385 Includes unit tests.
Implementation for https://wiki.php.net/rfc/static_return_type.
The type is only supported in covariant contexts (i.e. only return types right now), because it would be unsound anywhere else.
static
is considered a subtype ofself
.Because the
static
type conflicts syntactically withstatic
properties, it is excluded from property types on the grammar level.TheNow returnsstatic
is internally implemented as a builtin type because it has lots of magic semantics. Currently it is also exposed viaReflectionType::isBuiltin()
astrue
. However,static
is usually considered a class in userland, so it might make sense to hack that to returnfalse
?false
.